Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Introduce oauthLogin function #433

Merged
merged 13 commits into from
Jan 17, 2024
Merged

✨ Introduce oauthLogin function #433

merged 13 commits into from
Jan 17, 2024

Conversation

coyotte508
Copy link
Member

@coyotte508 coyotte508 commented Jan 5, 2024 •
edited
Loading

Fix #408

import { oauthLogin, oauthHandleRedirectIfPresent } from "@huggingface/hub";
import {HfInference} from "@huggingface/inference";

const oauthResult = await oauthHandleRedirectIfPresent();

if (!oauthResult) {
  // If the user is not logged in, redirect to the login page
  oauthLogin();
}

// You can use oauthResult.accessToken and oauthResult.userInfo
console.log(oauthResult);

const inference = new HfInference(oauthResult.accessToken);

await inference.textToImage({
  model: 'stabilityai/stable-diffusion-2',
  inputs: 'award winning high resolution photo of a giant tortoise/((ladybird)) hybrid, [trending on artstation]',
  parameters: {
    negative_prompt: 'blurry',
  }
})

Tested inside a space: https://huggingface.co/spaces/coyotte508/client-side-oauth

Mainly looking for reviews regarding the APIs / usability cc @xenova @radames @vvmnnnkv @jbilcke-hf @Wauplin .

I started with a single oauthLogin that was split into oauthLogin and oauthHandleRedirect with an extra oauthHandleRedirectIfPresent for convenience.

@coyotte508 coyotte508 added the hub @huggingface/hub related label Jan 5, 2024
@coyotte508 coyotte508 marked this pull request as ready for review January 9, 2024 16:18
@coyotte508 coyotte508 requested a review from xenova January 9, 2024 16:19
@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

mishig25
mishig25 reviewed Jan 9, 2024
View reviewed changes
packages/hub/README.md Outdated Show resolved Hide resolved
Wauplin
Wauplin reviewed Jan 10, 2024
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting read, thanks for the ping. DX-wise looks good to me. Just to be sure, is oauthHandleRedirect intended to be used or is it more like an internal helper and oauthHandleRedirectIfPresent is meant to be used more often?

Small comment in the documentation, I would mention accessTokenExpiresAt. In the Python+Gradio integration I think that the token expiration is one of the blocking aspect (or at least one frustrating aspect). Having it explicitly documented should help IMO (if was not the case in the gradio integration).

packages/hub/src/lib/oauth-handle-redirect.ts Show resolved Hide resolved
@radames
Copy link
Contributor

radames commented Jan 11, 2024

Overall the API looks good! can we test it already?

@coyotte508
Copy link
Member Author

coyotte508 commented Jan 11, 2024 •
edited
Loading

Took comments under consideration:

  • improved doc
  • renamed oauthLogin to oauthLoginUrl

Published a test version @huggingface/[email protected] @radames

import { oauthLoginUrl, oauthHandleRedirectIfPresent } from "https://esm.sh/@huggingface/[email protected]"

@coyotte508
Copy link
Member Author

Overall the API looks good! can we test it already?

https://huggingface.co/spaces/coyotte508/client-side-oauth uses the lib, checkout https://huggingface.co/spaces/coyotte508/client-side-oauth/blob/main/index.html

@radames
Copy link
Contributor

radames commented Jan 11, 2024

Overall the API looks good! can we test it already?

https://huggingface.co/spaces/coyotte508/client-side-oauth uses the lib, checkout https://huggingface.co/spaces/coyotte508/client-side-oauth/blob/main/index.html

cool, testing this https://huggingface.co/spaces/coyotte508/client-side-oauth , for some reason if I refresh the page I have to re-login to hf, but accessing it via https://coyotte508-client-side-oauth.static.hf.space/index.html I just need to click sign-in and no need to re-login, do you know why?

@coyotte508
Copy link
Member Author

Overall the API looks good! can we test it already?

huggingface.co/spaces/coyotte508/client-side-oauth uses the lib, checkout huggingface.co/spaces/coyotte508/client-side-oauth/blob/main/index.html

cool, testing this huggingface.co/spaces/coyotte508/client-side-oauth , for some reason if I refresh the page I have to re-login to hf, but accessing it via coyotte508-client-side-oauth.static.hf.space/index.html I just need to click sign-in and no need to re-login, do you know why?

Cookies storage inside iframes I guess, browser measures to prevent tracking

@coyotte508 coyotte508 merged commit e578fc3 into main Jan 17, 2024
1 of 2 checks passed
@coyotte508 coyotte508 deleted the oauthLogin branch January 17, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radames radames radames left review comments

@Wauplin Wauplin Wauplin left review comments

@mishig25 mishig25 mishig25 left review comments

@xenova xenova Awaiting requested review from xenova

Assignees
No one assigned
Labels
hub @huggingface/hub related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add signInWithHf() frontend helper
5 participants
@coyotte508 @HuggingFaceDocBuilderDev @radames @Wauplin @mishig25